-
Notifications
You must be signed in to change notification settings - Fork 104
Add interface abstractions for SchemaGenerator and Discoverer with DI improvements #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| * to improve performance when discovery is called multiple times. | ||
| * | ||
| * @internal | ||
| * @final |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I mark the class final directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, We will merge this PR in 0.3.0 anyways.
8179fb4 to
fd0aa5f
Compare
fd0aa5f to
9c63ce2
Compare
Nyholm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. I like to see more @internal and @final.
| * to improve performance when discovery is called multiple times. | ||
| * | ||
| * @internal | ||
| * @final |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, We will merge this PR in 0.3.0 anyways.
|
we should make a pass against all the classes of the sdk before tagging a major stable version to check usage of final/@internal I'm concerned about big diffs though so maybe I could prepare an issue with that? |
|
Sounds good to me :) |
|
WDYT about this one @chr-hertel could it be merged? |
chr-hertel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if this enables you to adopt the SDK in API Platform we should def get this in - makes sense, thanks! 👍
Only minor stuff and we could get it in :)
|
Actually this would make the use of other libraries so that each of them could provide bridges to work with the php-sdk: LLM generated examples for API Platform, Spiral/json-schema-generator or wol-soft/php-json-schema-model-generatorCustom Schema Generation ExamplesThe MCP PHP SDK now supports custom schema generators through the Why Use Custom Schema Generators?
Example 1: Using API Platform Schema FactoryAPI Platform provides a powerful schema factory for generating JSON Schemas from PHP classes with extensive metadata support. <?php
use ApiPlatform\JsonSchema\SchemaFactoryInterface;
use Mcp\Capability\Discovery\SchemaGeneratorInterface;
use Mcp\Server\Builder;
class ApiPlatformSchemaGenerator implements SchemaGeneratorInterface
{
public function __construct(
private SchemaFactoryInterface $schemaFactory,
) {}
public function generate(\Reflector $reflection): array
{
if (!$reflection instanceof \ReflectionMethod && !$reflection instanceof \ReflectionFunction) {
throw new \InvalidArgumentException('Only methods and functions are supported');
}
$properties = [];
$required = [];
foreach ($reflection->getParameters() as $parameter) {
$paramType = $parameter->getType();
// For complex types, use API Platform to generate schema
if ($paramType instanceof \ReflectionNamedType && !$paramType->isBuiltin()) {
$className = $paramType->getName();
$schema = $this->schemaFactory->buildSchema($className);
$properties[$parameter->getName()] = $schema['properties'] ?? [];
} else {
// Fallback to simple types
$properties[$parameter->getName()] = [
'type' => $this->mapPhpTypeToJsonSchema($paramType),
];
}
if (!$parameter->isOptional()) {
$required[] = $parameter->getName();
}
}
return [
'type' => 'object',
'properties' => $properties,
'required' => $required,
];
}
private function mapPhpTypeToJsonSchema(?\ReflectionType $type): string
{
if (!$type instanceof \ReflectionNamedType) {
return 'string';
}
return match ($type->getName()) {
'int' => 'integer',
'float' => 'number',
'bool' => 'boolean',
'array' => 'array',
default => 'string',
};
}
}
// Usage with Builder
$schemaFactory = /* get API Platform schema factory */;
$generator = new ApiPlatformSchemaGenerator($schemaFactory);
$server = Builder::create()
->setSchemaGenerator($generator)
->discover(__DIR__ . '/tools')
->build();Example 2: Using php-json-schema-model-generatorThe php-json-schema-model-generator library can generate schemas from PHP classes with docblocks and type hints. <?php
use PHPModelGenerator\SchemaGenerator\SchemaGeneratorInterface as PHPSchemaGeneratorInterface;
use Mcp\Capability\Discovery\SchemaGeneratorInterface;
use Mcp\Server\Builder;
class PHPModelSchemaGenerator implements SchemaGeneratorInterface
{
public function __construct(
private PHPSchemaGeneratorInterface $phpSchemaGenerator,
) {}
public function generate(\Reflector $reflection): array
{
if (!$reflection instanceof \ReflectionMethod && !$reflection instanceof \ReflectionFunction) {
throw new \InvalidArgumentException('Only methods and functions are supported');
}
$properties = [];
$required = [];
foreach ($reflection->getParameters() as $parameter) {
$paramType = $parameter->getType();
// If parameter is a class, generate its schema
if ($paramType instanceof \ReflectionNamedType && class_exists($paramType->getName())) {
$className = $paramType->getName();
// Generate a temporary JSON Schema file for the class
$tempSchema = $this->generateSchemaForClass($className);
$properties[$parameter->getName()] = $tempSchema;
} else {
// Simple type
$properties[$parameter->getName()] = [
'type' => $this->getJsonSchemaType($paramType),
];
}
if (!$parameter->isOptional()) {
$required[] = $parameter->getName();
}
}
return [
'type' => 'object',
'properties' => $properties,
'required' => $required,
];
}
private function generateSchemaForClass(string $className): array
{
// Use reflection to build a schema from class properties
$reflection = new \ReflectionClass($className);
$properties = [];
foreach ($reflection->getProperties(\ReflectionProperty::IS_PUBLIC) as $property) {
$type = $property->getType();
$properties[$property->getName()] = [
'type' => $this->getJsonSchemaType($type),
'description' => $this->extractDescription($property),
];
}
return [
'type' => 'object',
'properties' => $properties,
];
}
private function getJsonSchemaType(?\ReflectionType $type): string
{
if (!$type instanceof \ReflectionNamedType) {
return 'string';
}
return match ($type->getName()) {
'int' => 'integer',
'float', 'double' => 'number',
'bool' => 'boolean',
'array' => 'array',
default => 'string',
};
}
private function extractDescription(\ReflectionProperty $property): string
{
$docComment = $property->getDocComment();
if (!$docComment) {
return '';
}
// Simple extraction of description from docblock
preg_match('/@var\s+\S+\s+(.+)/', $docComment, $matches);
return $matches[1] ?? '';
}
}
// Usage
$phpSchemaGenerator = /* initialize php-json-schema-model-generator */;
$generator = new PHPModelSchemaGenerator($phpSchemaGenerator);
$server = Builder::create()
->setSchemaGenerator($generator)
->discover(__DIR__ . '/tools')
->build();Example 3: Spiral JSON Schema GeneratorThe spiral/json-schema-generator provides advanced schema generation with support for nested types, enums, and more. <?php
use Spiral\JsonSchema\Generator;
use Mcp\Capability\Discovery\SchemaGeneratorInterface;
use Mcp\Server\Builder;
class SpiralSchemaGenerator implements SchemaGeneratorInterface
{
public function __construct(
private Generator $spiralGenerator,
) {}
public function generate(\Reflector $reflection): array
{
if (!$reflection instanceof \ReflectionMethod && !$reflection instanceof \ReflectionFunction) {
throw new \InvalidArgumentException('Only methods and functions are supported');
}
$properties = [];
$required = [];
foreach ($reflection->getParameters() as $parameter) {
$paramType = $parameter->getType();
if ($paramType instanceof \ReflectionNamedType) {
// Use Spiral to generate schema for complex types
if (class_exists($paramType->getName()) || interface_exists($paramType->getName())) {
$schema = $this->spiralGenerator->generate($paramType->getName());
$properties[$parameter->getName()] = $schema;
} else {
$properties[$parameter->getName()] = [
'type' => $this->mapType($paramType->getName()),
];
}
} elseif ($paramType instanceof \ReflectionUnionType) {
// Handle union types with anyOf
$types = [];
foreach ($paramType->getTypes() as $type) {
if ($type instanceof \ReflectionNamedType) {
$types[] = ['type' => $this->mapType($type->getName())];
}
}
$properties[$parameter->getName()] = [
'anyOf' => $types,
];
}
if (!$parameter->isOptional()) {
$required[] = $parameter->getName();
}
}
return [
'type' => 'object',
'properties' => $properties,
'required' => $required,
];
}
private function mapType(string $typeName): string
{
return match ($typeName) {
'int' => 'integer',
'float', 'double' => 'number',
'bool' => 'boolean',
'array' => 'array',
'object' => 'object',
'null' => 'null',
default => 'string',
};
}
}
// Usage
$spiralGenerator = new Generator();
$generator = new SpiralSchemaGenerator($spiralGenerator);
$server = Builder::create()
->setSchemaGenerator($generator)
->discover(__DIR__ . '/tools')
->build();Benefits
Notes
|
chr-hertel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @soyuka!
Summary
Introduces interface-based abstractions for schema generation and discovery components, enabling extensibility and proper dependency injection patterns. This allows integration with external schema libraries (e.g., API Platform) and custom discovery implementations.
Motivation and Context
The current implementation tightly couples schema generation to PHP attributes and hardcodes discoverer instantiation within loaders. This makes it difficult to:
This PR decouples these concerns by introducing interfaces and moving object construction responsibilities to the Builder.
Breaking Changes
Minimal breaking changes:
SchemaGenerator::generate()now accepts\Reflectorinstead of\ReflectionMethod|\ReflectionFunctionDiscoveryLoaderconstructor signature changed (internal usage only, constructed by Builder)Users relying on default Builder behavior are not affected.
I suggest that the Discoverer (and probably the DiscoveryLoader) should be marked @internal and some of the classes be marked as
final. I'm unsure if you'd merge this changes as they may break current implementations, let me know.Types of changes
Checklist
Additional context
Changes:
SchemaGeneratorInterfacewithgenerate(\Reflector): arraymethodDiscovererInterface(marked@internal) withdiscover()methodSchemaGeneratornow implementsSchemaGeneratorInterfaceDiscovererandCachedDiscoverernow implementDiscovererInterface(marked@internaland@final)DiscoveryLoadersimplified to pure loader (no longer factory)Builder::setSchemaGenerator()andBuilder::setDiscoverer()for custom implementationsFuture possibilities:
ApiPlatformSchemaProvider)\BadMethodCallException) (note that currently a tool is defined using a method or a function but in the future we). open the possibility that a tool could be referenced as an invoke-able class or any other method).Also relates to #153 (indeed the #153 PR is missing the ArrayLoader change and looking at the code its unclear that the schema generator is used there as well as in the discoverer).